Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

executor: solve bug of copy joined tuples after inline projection #15411

Merged
merged 15 commits into from
Mar 20, 2020
Merged

executor: solve bug of copy joined tuples after inline projection #15411

merged 15 commits into from
Mar 20, 2020

Conversation

fzhedu
Copy link
Contributor

@fzhedu fzhedu commented Mar 16, 2020 •
edited
Loading

What problem does this PR solve?

Issue Number:
close #15316
close #15313
close #15312
close #15302
close #15299

Problem Summary: after taking inline projection, the outer part of a jointed rows may be pruned, so it cannot directly use chunk.CopySelectedJoinRowsWithSameOuterRows().

What is changed and how it works?

What's Changed: change the chunk.CopySelectedJoinRowsWithSameOuterRows() to solve the cases without outers.

How it Works: correctly copy inner part, and ignore the outer part.

Check List

Tests

  • Unit test

@fzhedu fzhedu requested a review from SunRunAway March 16, 2020 14:31
@fzhedu fzhedu requested a review from a team as a code owner March 16, 2020 14:31
@ghost ghost requested review from qw4990 and removed request for a team March 16, 2020 14:31
@fzhedu fzhedu added sig/execution SIG execution type/bugfix This PR fixes a bug. labels Mar 16, 2020
@fzhedu fzhedu requested a review from XuHuaiyu March 16, 2020 14:31
@XuHuaiyu XuHuaiyu closed this Mar 17, 2020
@XuHuaiyu XuHuaiyu reopened this Mar 17, 2020
@codecov
Copy link

codecov bot commented Mar 17, 2020 •
edited
Loading

Codecov Report

Merging #15411 into master will increase coverage by 0.1747%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master     #15411        +/-   ##
================================================
+ Coverage   80.3898%   80.5645%   +0.1747%     
================================================
  Files           502        502                
  Lines        134058     134872       +814     
================================================
+ Hits         107769     108659       +890     
+ Misses        17819      17771        -48     
+ Partials       8470       8442        -28

@fzhedu fzhedu requested a review from XuHuaiyu March 17, 2020 10:05
@fzhedu
Copy link
Contributor Author

fzhedu commented Mar 17, 2020

/run-unit-test

SunRunAway
SunRunAway reviewed Mar 17, 2020
View reviewed changes
executor/join_test.go Show resolved Hide resolved
executor/joiner.go Outdated Show resolved Hide resolved
util/chunk/chunk_util_test.go Show resolved Hide resolved
SunRunAway
SunRunAway reviewed Mar 17, 2020
View reviewed changes
executor/join_test.go Show resolved Hide resolved
executor/join_test.go Show resolved Hide resolved
@fzhedu fzhedu requested a review from a team as a code owner March 18, 2020 05:58
@ghost ghost requested review from eurekaka and winoros and removed request for a team March 18, 2020 05:58
@fzhedu fzhedu requested a review from SunRunAway March 18, 2020 06:07
@fzhedu fzhedu requested a review from SunRunAway March 18, 2020 08:38
SunRunAway
SunRunAway reviewed Mar 18, 2020
View reviewed changes
executor/joiner.go Outdated Show resolved Hide resolved
SunRunAway
SunRunAway reviewed Mar 18, 2020
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM
LGTM

@SunRunAway SunRunAway added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 18, 2020
@fzhedu
Copy link
Contributor Author

fzhedu commented Mar 18, 2020

/rebuild

@SunRunAway SunRunAway closed this Mar 18, 2020
@SunRunAway SunRunAway reopened this Mar 18, 2020
XuHuaiyu
XuHuaiyu approved these changes Mar 20, 2020
View reviewed changes
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@XuHuaiyu XuHuaiyu added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 20, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 20, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 20, 2020

@fzhedu merge failed.

@fzhedu fzhedu changed the title executor: solve bug for inline projection executor: solve bug of copy joined tuples after inline projection Mar 20, 2020
@fzhedu
Copy link
Contributor Author

fzhedu commented Mar 20, 2020

/rebuild

@fzhedu
Copy link
Contributor Author

fzhedu commented Mar 20, 2020

/run-all-tests

@fzhedu
Copy link
Contributor Author

fzhedu commented Mar 20, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Mar 20, 2020

/run-all-tests

@sre-bot sre-bot merged commit b0ea766 into pingcap:master Mar 20, 2020
@you06 you06 added this to the v4.0.0-rc milestone Mar 27, 2020
@qw4990
Copy link
Contributor

qw4990 commented Sep 8, 2020

/run-cherry-picker

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 8, 2020
@fzhedu @ti-srebot
cherry pick pingcap#15411 to release-4.0
34ab6b2 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Sep 8, 2020
Closed
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #19870

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SunRunAway SunRunAway SunRunAway left review comments

@XuHuaiyu XuHuaiyu XuHuaiyu approved these changes

@qw4990 qw4990 Awaiting requested review from qw4990

@eurekaka eurekaka Awaiting requested review from eurekaka

@winoros winoros Awaiting requested review from winoros

Assignees
No one assigned
Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
v4.0.0-rc
7 participants
@fzhedu @sre-bot @qw4990 @ti-srebot @SunRunAway @XuHuaiyu @you06